Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

switchroot: Move late /run/ostree-booted creation to ostree-system-ge… #1675

Closed
wants to merge 1 commit into from

Conversation

akiernan
Copy link
Contributor

@akiernan akiernan commented Jul 7, 2018

…nerator

When ostree-prepare-root is pid 1, ostree-prepare-boot defers creation of
/run/ostree-booted, which we were doing in ostree-remount, but that's too
late if we need ostree-system-generator to bind /var. Move the creation
of the /run/ostree-booted marker to ostree-system-generator based on the
existence of the ostree= kernel command line argument (which matches the
condition that ostree-remount used).

Signed-off-by: Alex Kiernan [email protected]

@cgwalters
Copy link
Member

Something I wasn't clear on, would this need changing to EXIT_SUCCESS if this patch moves forward?

Definitely; see https://bugzilla.redhat.com/show_bug.cgi?id=1331369 While we could have more clearly split out the ostree-as-host portions in the Fedora packaging, people can easily get those bits installed as a dependency of flatpak, etc. I think the Debian packager was more careful about this, but it still pays to be defensive.

Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with two changes.

* a file versus parsing the kernel cmdline. So let's ensure
* the stamp file is created here too.
*/
touch_run_ostree ();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been trying at least a little bit to support the non-systemd case. After this change in the "switchroot-as-pid1, !systemd" case nothing will be creating /run/ostree-booted right? It should be harmless to leave this call in to ensure we create the file the same as we did previously.

Maybe just:

/* When systemd is in use this is normally created via the generator, but
 * we ensure it's created here as well for redundancy.
 */
  touch_run_ostree ();

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that makes sense... when I deleted it from there I debated whether creating /run/ostree-booted in many places was a sensible approach to cover corner cases. I'm certainly guilty of not thinking about !systemd...

@akiernan akiernan force-pushed the us-ostree-booted branch from 587bb67 to ea27e5a Compare July 8, 2018 14:54
…nerator

When ostree-prepare-root is pid 1, ostree-prepare-boot defers creation of
/run/ostree-booted, which happens in ostree-remount, but that's too late
if we need ostree-system-generator to bind /var. Add the creation of the
/run/ostree-booted marker to ostree-system-generator based on the
existence of the ostree= kernel command line argument (which matches the
condition that ostree-remount uses).

Signed-off-by: Alex Kiernan <[email protected]>
@akiernan akiernan force-pushed the us-ostree-booted branch from ea27e5a to ae36f71 Compare July 8, 2018 21:35
@cgwalters
Copy link
Member

Looks great, thank you!

@rh-atomic-bot r+ ae36f71

@rh-atomic-bot
Copy link

⚡ Test exempted: pull fully rebased and already tested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants